Skip to content

Removing NO_AUTO_PTR as its a cause of warning generation and not needed#45

Open
rolesen wants to merge 2 commits intomasterfrom
rlo_no_auto_ptr
Open

Removing NO_AUTO_PTR as its a cause of warning generation and not needed#45
rolesen wants to merge 2 commits intomasterfrom
rlo_no_auto_ptr

Conversation

@rolesen
Copy link

@rolesen rolesen commented Nov 23, 2020

No description provided.

@rolesen rolesen requested review from jpihl and mortenvp November 23, 2020 12:50
@mortenvp
Copy link
Member

I guess it is enough to just define it? Does it need the empty value?

@rolesen
Copy link
Author

rolesen commented Nov 26, 2020 via email

@mortenvp
Copy link
Member

Simply defining it should make it work with #ifdef etc., are they doing something else with that marco (like checking it's value)?

@rolesen
Copy link
Author

rolesen commented Nov 28, 2020 via email

@mortenvp
Copy link
Member

Ok, just saw you wrote that the default value of 1 gives warnings. Where is that happening? In Thrift?

@rolesen
Copy link
Author

rolesen commented Nov 28, 2020 via email

@mortenvp
Copy link
Member

I just to get what kind of warning that produces.. since most code just checks whether it is defined e.g. with #ifdef

@rolesen
Copy link
Author

rolesen commented Nov 28, 2020 via email

@mortenvp
Copy link
Member

So this means that we are not the only ones defining it? Where is it being defined (besides by us)?

@rolesen
Copy link
Author

rolesen commented Nov 29, 2020 via email

@mortenvp
Copy link
Member

I see, in that case would the right solution not be to ensure that it isn't defined twice by guarding that #define with and #ifndef?

@mortenvp
Copy link
Member

For example here BOOST should probably check whether that macro has already been defined:
https://github.com/boostorg/config/blob/a9bc1346109c69bc2b5c7eb8251794c9278c7a31/include/boost/config/stdlib/libstdcpp3.hpp#L215

Because on older compilers they do not define it - but there it will cause deprecation warnings.. So to have have it both on old and new you need to provide it yourself. Unless you want a complicated build script where you check the version of the compiler and then optionally provide it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants